fix(mcp): recover cleanly from OAuth failures by j15z · Pull Request #5595 · simstudioai/sim

j15z · 2026-07-11T17:54:01Z

Summary

This PR makes MCP authentication and refresh failures recover cleanly instead of leaving Workspace Settings in repeated discovery loops or generic error states. Abandoned OAuth servers now wait for an explicit reconnect, failed refreshes expose a retryable Failed state, and persisted connection errors reach the list and detail views.

Dual-auth servers can now be added with a configured static Authorization: Bearer ... header: the connection test tries that credential before optional OAuth discovery. Supplemental non-auth headers still follow OAuth discovery, preserving OAuth servers that need routing or metadata headers.

Connection attempts honor the configured timeout, and diagnostics record timing, phase, outcome, auth type, and header names without header values. Connection-test failures now use allowlisted messages so an upstream server cannot reflect a configured credential into API responses or logs. OAuth-pending servers remain disconnected without negative caching; other failures promote to error after repeated attempts and reset after successful discovery.

Type of Change

Testing

The original PR validation passed 25 focused MCP/API test files (375 tests) and the React Query pattern audit.
The static Bearer/OAuth update passed 8 focused MCP/API/security test files (123 tests), including valid and rejected Bearer credentials, headerless OAuth, OAuth with supplemental headers, post-resolution SSRF blocking, and reflected-secret handling.
API validation audit passed with all 927 routes Zod-backed.
Biome and git diff --check passed for the changed files.
Full TypeScript checking still reports an unrelated existing error in providers/meta/index.ts; the changed paths compile through the focused Vitest runs.

Review focus: static Bearer precedence, preservation of OAuth-plus-supplemental-header behavior, and the allowlisted connection-error boundary.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Post-Deploy Monitoring & Validation

Search application logs for McpClient, McpService, authorization_required, timeout, and OAuth authorization pending during the first 24 hours.
Healthy signals: abandoned OAuth servers stay disconnected without repeated discovery failures; static Bearer dual-auth servers connect; successful reconnects clear failure counters; no header values appear in MCP diagnostics.
Failure signals: renewed discovery loops, static Bearer servers being redirected into OAuth, OAuth servers with supplemental headers failing connection tests, or credential fragments appearing in responses or logs.
Roll back if MCP list availability regresses or refresh failures stop exposing a safe actionable error. Owner: MCP/settings team during the first business day after deployment.

Screenshots/Videos

Not applicable; this changes connection behavior and failure handling without altering layout.

vercel · 2026-07-11T17:54:06Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jul 14, 2026 6:03pm

cursor · 2026-07-11T17:54:11Z

PR Summary

Medium Risk
Touches MCP auth, connection testing, credential handling in errors/logs, and persisted server status—important for security and UX but covered by extensive new tests.

Overview
Fixes MCP settings behavior when OAuth is abandoned or discovery fails, so the UI stops hammering doomed requests and shows accurate connection state.

Service and APIs: Discovery failures are persisted in mcpService (OAuth-required → disconnected with no lastError and no negative cache; other failures increment consecutiveFailures and promote to error after repeated failures). Status updates can be skipped when a newer successful connection wins a race. The refresh route no longer overwrites connectionStatus / lastError—it only bumps lastToolsRefresh and returns whatever the service already stored. Test-connection prefers a static Authorization: Bearer over OAuth discovery, maps errors to an allowlist so upstream echoes cannot leak credentials, and blocks SSRF before sending headers.

Client and queries: McpClient passes the configured timeout into SDK initialize and logs structured diagnostics (header names only, sanitized errors). React Query skips tool discovery for disconnected OAuth servers, refetches the server list when discovery fails, and excludes those servers from force-refresh.

Settings UI: Shared helpers drive tool-count labels (Not Connected vs persisted errors) and refresh chip state (Failed in red, retryable vs Synced). Settings header actions support textTone: 'error'.

^{Reviewed by Cursor Bugbot for commit c0bd4e4. Bugbot is set up for automated code reviews on this repo. Configure here.}

greptile-apps · 2026-07-11T17:56:11Z

Greptile Summary

This PR improves MCP OAuth recovery and refresh feedback. The main changes are:

Skips automatic tool discovery for OAuth servers waiting on reconnect.
Persists MCP discovery failures and refresh results for the settings UI.
Adds clearer refresh button states and server error labels.
Adds connection diagnostics with timing, phase, outcome, auth type, and header names.

Confidence Score: 4/5

The changed MCP failure paths need fixes before merging.

Non-OAuth 401 responses can be persisted as OAuth-pending with no actionable error.
New diagnostic logging can expose secret prefixes from truncated external error text.
The UI-only refresh state changes look contained.

apps/sim/lib/mcp/service.ts and apps/sim/lib/mcp/client.ts

Security Review

The new MCP diagnostics can log truncated external error text before redaction has safely removed secret fragments.

Important Files Changed

Filename	Overview
apps/sim/lib/mcp/service.ts	Adds MCP status persistence and OAuth-pending handling, but the shared auth-error helper can hide non-OAuth 401 credential failures.
apps/sim/lib/mcp/client.ts	Adds connect timeout handling and structured diagnostics, but the new error logging can expose truncated secret fragments.
apps/sim/hooks/queries/mcp.ts	Stops auto-discovery for disconnected OAuth servers while keeping non-OAuth discovery behavior.
apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx	Uses helper functions for MCP refresh action state and server tool labels.
apps/sim/app/workspace/[workspaceId]/settings/components/settings-header/settings-header.tsx	Adds optional error-tone rendering for settings header actions.

_{Reviews (1): Last reviewed commit: "fix(mcp): improve OAuth failure recovery" | Re-trigger Greptile}

greptile-apps · 2026-07-11T17:56:14Z

+function isOauthAuthorizationError(error: unknown): boolean {
+  return error instanceof McpOauthAuthorizationRequiredError || error instanceof UnauthorizedError
+}


Non-OAuth 401s Lose Errors

When a headers-auth MCP server returns 401, the SDK can surface UnauthorizedError, but this helper now routes that through the OAuth-pending path. That path clears lastError, skips the failure counter, and avoids the failure cache, so disconnected non-OAuth servers can be retried repeatedly while the UI has no persisted credential error to show.

greptile-apps · 2026-07-11T17:56:15Z

+          ...describedError,
+          message: sanitizeForLogging(describedError.message, 500),
+          causeChain: describedError.causeChain?.map((cause) => sanitizeForLogging(cause, 500)),
+          stack:
+            error instanceof Error && error.stack
+              ? sanitizeForLogging(error.stack.split('\n').slice(1).join('\n'), 2000)
+              : undefined,


Truncated Secrets Can Leak

These new diagnostics log external MCP error messages, cause chains, and stacks after sanitizeForLogging, but that helper truncates before redacting. If a server error echoes a long or unterminated secret-bearing fragment, the redaction patterns may not match the truncated text and a secret prefix can be written to structured logs.

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit c0bd4e4. Configure here.}

cursor · 2026-07-14T18:05:33Z

            updatedAt: now,
          })
-          .where(eq(mcpServers.id, serverId))
-          .returning()


Refresh masks discovery failures

Medium Severity

When discoverServerTools throws, the handler still builds the HTTP response from the post-refresh database row. If persistence was skipped (for example the new discoveryStartedAt guard or a failed status write) while the row still shows connected, the API returns a successful connected refresh with a cleared error even though discovery failed and tool sync was skipped.

^{Reviewed by Cursor Bugbot for commit c0bd4e4. Configure here.}

greptile-apps Bot reviewed Jul 11, 2026

View reviewed changes

cursor Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/hooks/queries/mcp.ts Outdated

fix(mcp): improve OAuth failure recovery

e72a777

j15z force-pushed the fix/mcp-page branch from ef5f993 to e72a777 Compare July 11, 2026 22:37

vercel Bot temporarily deployed to Preview July 11, 2026 22:37 Inactive

fix(mcp): address OAuth recovery review findings

97b3e73

vercel Bot temporarily deployed to Preview July 11, 2026 23:22 Inactive

fix(mcp): prefer static bearer auth in connection tests

c0bd4e4

vercel Bot temporarily deployed to Preview July 14, 2026 18:03 Inactive

cursor Bot reviewed Jul 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(mcp): recover cleanly from OAuth failures#5595

fix(mcp): recover cleanly from OAuth failures#5595
j15z wants to merge 3 commits into
stagingfrom
fix/mcp-page

j15z commented Jul 11, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Jul 11, 2026 •

edited

Loading

Uh oh!

cursor Bot commented Jul 11, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jul 11, 2026

Security Review

Uh oh!

greptile-apps Bot Jul 11, 2026

Uh oh!

greptile-apps Bot Jul 11, 2026

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

j15z commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of Change

Testing

Checklist

Post-Deploy Monitoring & Validation

Screenshots/Videos

Uh oh!

vercel Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

greptile-apps Bot commented Jul 11, 2026

Greptile Summary

Confidence Score: 4/5

Security Review

Important Files Changed

Uh oh!

greptile-apps Bot Jul 11, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jul 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 14, 2026

Choose a reason for hiding this comment

Refresh masks discovery failures

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

j15z commented Jul 11, 2026 •

edited

Loading

vercel Bot commented Jul 11, 2026 •

edited

Loading

cursor Bot commented Jul 11, 2026 •

edited

Loading